-
Notifications
You must be signed in to change notification settings - Fork 123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: Fix is_not_rw_storage #2595
Conversation
Thank you for working on this! Seems like you already solved follow-up problems that turned up after you fixed the function? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me (apart from the failing tests)
@markus2330 Yes to get the tests to run again, I did manually exclude the following plugins from
|
I added test files for |
YAMBi (Yan LR, YAwn, and YAy PEG) do not include write support. They all use YAML Smith to write data back to a YAML file.
Are you sure about that? I tried the following, which seems to work: kdb set user/tests/key value
kdb export user/tests yamlcpp
#> key: value
kdb export user/tests/key yamlcpp
#> value . |
The problem I encountered with yamlcpp is that like i.e.
gives
but if you omit setting |
Thank you for the clarification. For this purpose YAML CPP requires the Directory Value plugin. Unfortunately |
To get this PR ready, I think we need to exclude these plugins now. But please mark that they should be readded (directly next to the list and in doc/todo/TESTING).
yajl should now be able to do so. The PR was merged recently. |
Any progress here? |
I'll do it once I've finished with #3102. (no PR yet, the code is still quite chaotic and broken) |
is_not_rw_storage now checks if the output of "kdb info {plugin} provides" contains the word "storage" instead of checking if it is equal to storage. ElektraInitiative#2423
check_export did not expect the quotes around the value in the output of "kdb set". It now does. ElektraInitiative#2423
@markus2330 I rebased the branch, let's see what the build servers say... |
I added the test data for @bauhaus93 You should probably try to fix the storage tests for the |
Great job! 💖 |
Basics
Check relevant points but please do not remove entries.
Do not describe the purpose of this PR in the PR description but:
doc/news/_preparation_next_release.md
which contains_(my name)_
)Please always add something to the the release notes.
(first line should have
module: short statement
syntax)close #X
, should be in the commit messages.Checklist
Check relevant points but please do not remove entries.
For docu fixes, spell checking, and similar none of these points below
need to be checked.
Review
Remove the line below and add the "work in progress" label if you do
not want the PR to be reviewed:
@markus2330 please review my pull request
Merging
Please add the "ready to merge" label when the build server and you say
that everything is ready to be merged.